refactor: introduce FunctionCallbackResolver interface #1804
Conversation
| * @param functionCallbackContext the function callback context used to store the | ||
| * state of the function calls. | ||
| * @param functionCallbackResolver the function callback resolver used to resolve the | ||
| * function by bean name.s |
| * Moonshot Chat API. | ||
| * @param options The MoonshotChatOptions to configure the chat client. | ||
| * @param functionCallbackContext The function callback context. | ||
| * @param functionCallbackResolver The function callback resolver to |
There was a problem hiding this comment.
javadoc sentence incomplete.
| * Chat API. | ||
| * @param options The OpenAiChatOptions to configure the chat model. | ||
| * @param functionCallbackContext The function callback context. | ||
| * @param functionCallbackResolver The function callback context. |
There was a problem hiding this comment.
javadoc still refers to callback context.
|
|
||
| var chatTypes = Set.of(AbstractMessage.class, AssistantMessage.class, ToolResponseMessage.class, Message.class, | ||
| MessageType.class, UserMessage.class, SystemMessage.class, FunctionCallbackContext.class, | ||
| MessageType.class, UserMessage.class, SystemMessage.class, DefaultFunctionCallbackResolver.class, |
There was a problem hiding this comment.
How does this work when we have other implementations of "FunctionCallbackResolver"
There was a problem hiding this comment.
If there are other implementation they have to be added here.
| else { | ||
| throw new IllegalStateException( | ||
| "No function callback [" + functionName + "] fund in tht FunctionCallbackContext"); | ||
| "No function callback [" + functionName + "] fund in tht FunctionCallbackRegister"); |
There was a problem hiding this comment.
typo in the exception message.
| import org.springframework.lang.Nullable; | ||
|
|
||
| /** | ||
| * Strategy interface for resolving {@link FunctionCallback} instances as bean from the |
There was a problem hiding this comment.
I believe there could be an implementation for the FunctionCallbackResolver that doesn't depend on the application context, right?
There was a problem hiding this comment.
I can not think of any not app context case?
- Introduces a new FunctionCallbackResolver interface to define the strategy for
resolving FunctionCallback instances from the application context.
- Renames FunctionCallbackContext to DefaultFunctionCallbackResolver to better reflect its
implementation role. Updates all related components to use the new interface.
- Update the affected AI model implementations
- Replaces FunctionCallbackContext parameter with FunctionCallbackResolver in all
model constructors
- Updates builder patterns to use functionCallbackResolver() method instead of
withFunctionCallbackContext()
- Deprecates old withFunctionCallbackContext() methods in builders to guide
migration
- Updates integration tests to use DefaultFunctionCallbackResolver
- Improves documentation to clarify the resolver's role in function callbacks
- Moves SchemaType enum from FunctionCallbackContext to FunctionCallback (Braking change)git add .
Resolves spring-projects#758
tzolov
force-pushed
the
issue-758-function-callback-resolver
branch
from
33c25ee to
30af292
Compare
| * @param beanName the name of the bean to resolve | ||
| * @return the {@link FunctionCallback} instance | ||
| */ | ||
| FunctionCallback getFunctionCallback(@NonNull String beanName, @Nullable String defaultDescription); |
There was a problem hiding this comment.
what about
FunctionCallback resolve(@NonNull String name)
The description should be provided by the implementation.
| * @param functionCallbackContext the function callback context used to store the | ||
| * state of the function calls. | ||
| * @param functionCallbackResolver the function callback resolver used to resolve the | ||
| * function by bean name |
There was a problem hiding this comment.
Javadoc doesn't need to refer "bean" and just "by name" is enough. will fix when merging.
|
Fixed the javadoc, squashed, rebased and merged as 479a095 |
|
Also, fixed an issue with the AnthropicAutoConfiguration's AnthropicChatModel to use the interface dependent type |
3 participants
Resolves #758